Skip to content

fix(steps): add missing props, fix a11y, replace hardcoded colors#8507

Merged
tangjinzhou merged 3 commits intovueComponent:feat/vaporfrom
Quinn-Qingyu:review/steps
Apr 3, 2026
Merged

fix(steps): add missing props, fix a11y, replace hardcoded colors#8507
tangjinzhou merged 3 commits intovueComponent:feat/vaporfrom
Quinn-Qingyu:review/steps

Conversation

@Quinn-Qingyu
Copy link
Copy Markdown

@Quinn-Qingyu Quinn-Qingyu commented Mar 24, 2026

Summary

  • types.ts: Add missing progressDot, items, responsive, inline type, icon prop, ProgressDotRender type, StepItem interface; remove size default to allow ConfigProvider inheritance
  • Steps.vue: Integrate useConfigInject for global size, useBreakpoint for responsive auto-vertical; support items prop via v-for; add inline/dot CSS classes; pass initial/progressDot/type via context
  • Step.vue: Conditional role="button" only when clickable; support icon prop rendering; progressDot dot icon; initial offset in step numbering
  • style/index.css: Replace 3x hardcoded #fff with CSS variables for dark theme support; add progressDot and inline type styles
  • tests: Add Inline demo to demo.test.ts; add 7 new test cases (inline type, progressDot, items prop, initial offset, dot icon, conditional role, component names)

Test plan

  • All 54 tests pass (38 unit + 16 demo snapshots)
  • No linter errors introduced
  • Snapshots regenerated

Note: responsive defaults to true (matching Ant Design React), so horizontal Steps switch to vertical on xs screens. Consumers can set :responsive="false" to opt out.

- types.ts: add progressDot, items, responsive, inline type, icon prop,
  ProgressDotRender type, StepItem interface, remove size default to
  allow ConfigProvider inheritance
- Steps.vue: integrate useConfigInject for global size, useBreakpoint
  for responsive auto-vertical, support items prop via v-for, add
  inline/dot CSS classes, pass initial/progressDot/type via context
- Step.vue: conditional role="button" only when clickable, support icon
  prop rendering, progressDot dot icon, initial offset in step numbering
- style/index.css: replace 3x hardcoded #fff with CSS variables for
  dark theme support, add progressDot and inline type styles
- tests: add Inline demo to demo.test.ts, add 7 new test cases for
  inline type, progressDot, items prop, initial offset, dot icon,
  conditional role, component names

Co-Authored-By: Cursor <noreply@cursor.com>
Made-with: Cursor
@Quinn-Qingyu
Copy link
Copy Markdown
Author

Note: All 4 CI failures are pre-existing on the feat/vapor branch and unrelated to this PR:

  • Lint: errors in apps/playground/vite.config.ts (empty block statements)
  • Test: calendar demo snapshot mismatch (date-dependent)
  • Type Check: configured with continue-on-error: true
  • PR Preview: fails due to upstream build issues

Steps component tests all pass locally (53/53).

@tangjinzhou
Copy link
Copy Markdown
Member

Review: fix(steps): add missing props, fix a11y, replace hardcoded colors

Good work overall — the a11y fix (conditional role="button"), CSS variable replacement for dark theme, and items prop support are solid additions. Here's detailed feedback:


Must fix

1. icon prop renders without .ant-steps-icon wrapper — styling bug

In Step.vue, the icon-prop branch renders bare <component :is> without the .ant-steps-icon span:

<component v-if="hasCustomIcon && icon" :is="icon as Component" />
<span v-else-if="isProgressDot" class="ant-steps-icon ant-steps-icon-dot" />
<span v-else class="ant-steps-icon">...</span>

The slot-based and number/check/close branches all have the .ant-steps-icon wrapper, but the icon prop path doesn't. This means the custom icon won't pick up any of the .ant-steps-icon styles (color, font-size, line-height, etc). Wrap it:

<span v-if="hasCustomIcon && icon" class="ant-steps-icon">
  <component :is="icon as Component" />
</span>

2. ProgressDotRender function type defined but never implemented

types.ts defines ProgressDotRender and the progressDot slot in StepsSlots, and StepsContextType carries progressDot: Ref<boolean | ProgressDotRender | undefined>. But neither Steps.vue nor Step.vue actually handle the function case — Step.vue just does !!context?.progressDot.value which would treat a function as truthy and render the default dot, ignoring the custom render.

Options:

  • a) Remove ProgressDotRender and the progressDot slot for now, keep progressDot as boolean only, and add function support in a follow-up PR.
  • b) Implement it now — Step.vue should call the function or use the scoped slot.

I'd recommend (a) to keep this PR focused.

3. items v-for key uses item.title ?? index — duplicate key risk

<Step v-for="(item, index) in (items || [])" :key="item.title ?? index" .../>

If two items have the same title (or both have undefined title), they'll share the same Vue key, causing rendering bugs. Just use :key="index" — it's stable and items are static.


Should fix

4. icon type Component | string — string case has no meaningful handling

StepItem.icon and StepProps.icon are typed as Component | string. But <component :is="icon as Component"> with a string would render a native HTML element by that name (e.g. "div"<div/>), which isn't useful for steps. If string icons aren't supported, narrow the type to just Component. If you want string support (icon name lookup), implement it.

5. CSS hardcoded margin-inline-start values in progressDot styles

.ant-steps-dot .ant-steps-item-icon { margin-inline-start: 67px; }
.ant-steps-dot .ant-steps-item-process .ant-steps-item-icon { margin-inline-start: 66px; }
.ant-steps-dot .ant-steps-item-tail { margin: 0 0 0 70px; }

These magic numbers are brittle and depend on .ant-steps-item-content having exactly width: 140px. Consider using calc() or at minimum add a comment explaining the derivation.


Consider

6. responsive: true as default

This matches Ant Design React's behavior, so it's fine — but it means all existing horizontal Steps will auto-switch to vertical on xs screens. Worth noting in the PR description in case anyone hits this.

7. mergedSize mapping only handles sm

const mergedSize = computed(() => props.size ?? (globalSize.value === 'sm' ? 'small' : 'default'))

ConfigProvider size can be 'sm' | 'md' | 'lg', but only smsmall is mapped. Both md and lg fall to 'default'. This is probably intentional (Steps only has default/small), but a comment would clarify that lg/md both map to default.


Positive

  • Conditional role="button" — great a11y improvement. Non-clickable steps shouldn't announce as buttons.
  • CSS variable replacement (#fffvar(--color-neutral-bg-container, #fff), var(--color-accent-content, #fff)) — proper dark theme support with sensible fallbacks.
  • useConfigInject + useBreakpoint integration — follows the project's pattern for global config.
  • Good test coverage — 7 new test cases covering inline, progressDot, items, initial offset, dot icon, conditional role, and component names.
  • initial prop offset for step numbering display is correctly implemented.

… fix

- Wrap icon prop in .ant-steps-icon span for proper styling (vueComponent#1)
- Remove unimplemented ProgressDotRender type, keep progressDot as boolean (vueComponent#2)
- Use index as v-for key to avoid duplicate key risk (vueComponent#3)
- Narrow icon type from Component | string to Component (vueComponent#4)
- Add comments for CSS magic numbers and mergedSize mapping (vueComponent#5, vueComponent#7)
- Add regression test for icon prop wrapper with markRaw

Made-with: Cursor
@Quinn-Qingyu
Copy link
Copy Markdown
Author

Addressed the review comments:

  1. Wrapped the icon prop render path with .ant-steps-icon so custom icons receive the same styling as the other branches.
  2. Removed the unimplemented ProgressDotRender / progressDot slot typing and kept progressDot as boolean only. Function support can be added in a follow-up PR if needed.
  3. Changed the items rendering key from item.title ?? index to index.
  4. Narrowed icon typing from Component | string to Component.
  5. Added comments explaining the progress-dot offset values.
  6. Added a comment clarifying that ConfigProvider md / lg both map to default because Steps only supports default and small.

Also added a regression test for the icon prop wrapper case.

Verified locally: 54 tests passing across index.test.ts and demo.test.ts.

@tangjinzhou
Copy link
Copy Markdown
Member

Review: fix(steps): add missing props, fix a11y, replace hardcoded colors

Thanks for the solid contribution! I reviewed both commits against the project conventions in CLAUDE.md. Tests pass (45/45). Here is my feedback:


Positive

  • A11y fix for role="button" — Making the role conditional via stepRole is correct. Non-clickable steps should not have role="button". Good catch.
  • Hardcoded colors replaced with CSS variablescolor: #fffvar(--color-accent-content, #fff) and background: #fffvar(--color-neutral-bg-container, #fff). This is exactly what the project conventions require.
  • CSS follows conventions — All new styles use :where() for low specificity, CSS variables with fallbacks, and @apply for Tailwind utilities. The magic number comments in the progress-dot section are a nice touch.
  • items prop implementation — Clean slot fallback pattern with <slot> wrapping the v-for render. Using index as key (from the review follow-up commit) is the right call.
  • initial prop for display numbering — Correct: initial offsets the displayed number without affecting status logic. Test coverage confirms this.
  • useConfigInject / useBreakpoint integrationmergedSize properly maps ConfigProvider sizes to Steps' 'default' | 'small', and mergedDirection handles responsive fallback to vertical on xs screens. SSR-safe since useBreakpoint returns an empty map until mounted.
  • StepItem interface — Clean data type that mirrors StepProps for the declarative items API.
  • Test coverage — Good additions: items prop rendering, initial offset, progressDot dot icon, icon prop wrapper, inline/dot class assertions, component name checks.

Must Fix

1. Inline demo does not use type="inline"

The demo/inline.vue file uses size="small" with manual <a-step> children — it does not actually demonstrate the type="inline" feature at all. It should use type="inline" with the items prop, similar to the antd React docs:

<a-steps
  type="inline"
  :current="item.current"
  :status="item.status"
  :items="items"
/>

The demo that was on feat/vapor before this PR (using <a-list> with type="inline" and :items) was actually correct. It appears the review follow-up commit inadvertently replaced it with a non-inline version.

2. Demo snapshot needs updating after fixing inline demo

After fixing the demo, run npx vitest run packages/ui/src/components/steps --update to regenerate the snapshot for the Inline demo entry.


Should Fix

3. Trailing blank line in Steps.vue

There is an extra blank line before </script> on line 67 of Steps.vue. Minor style nit — just remove it for consistency.


Consider

4. hasCustomIcon interaction with progressDot

When progressDot is true AND props.icon is set on a step (e.g., via items), hasCustomIcon will be true, adding ant-steps-item-custom class which removes border/background from the icon. The template correctly skips the icon rendering in favor of the dot (v-else-if="isProgressDot"), but the ant-steps-item-custom class still gets applied. This is unlikely to cause issues in practice since progressDot + icon on the same step is an unusual combination, but you could guard it:

const classes = computed(() => ({
  ...
  'ant-steps-item-custom': hasCustomIcon.value && !isProgressDot.value,
}))

5. Missing comment on stepIndex assignment

The original // Register this step and get its index comment was removed. Consider keeping it — it explains the non-obvious side-effect of registerStep().


Overall this is a high-quality PR. The inline demo is the only blocking issue — once fixed, this is ready to merge. Thanks!

- Rewrite inline demo to use type="inline" with :items prop
- Update demo snapshot for inline type
- Guard ant-steps-item-custom class when progressDot is active
- Add comment on stepIndex side-effect
- Remove trailing blank line in Steps.vue

Made-with: Cursor
@Quinn-Qingyu
Copy link
Copy Markdown
Author

Thanks for the detailed review! All points addressed:

  1. Rewrote the inline demo to properly use type="inline" with :items — good catch, the previous version was missing the key feature.
  2. Updated the demo snapshot accordingly.
  3. Removed the trailing blank line in Steps.vue.
  4. Added a guard on ant-steps-item-custom so it won't apply when progressDot is active — makes sense to avoid the style conflict.
  5. Restored the comment on stepIndex to clarify the registerStep() side-effect.

All 54 tests passing locally. Let me know if anything else needs adjusting!

@tangjinzhou tangjinzhou merged commit 321b38f into vueComponent:feat/vapor Apr 3, 2026
@Quinn-Qingyu
Copy link
Copy Markdown
Author

Thanks @tangjinzhou for the thorough reviews and for merging! Learned a lot from the feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants